Cap total number of concurrent commands per connection #592

arturobernalg · 2025-12-27T18:14:06Z

This change adds an optional hard cap on the number of pending request execution commands queued per HTTP/2 connection. When the per-connection limit is reached, new submissions fail fast with RejectedExecutionException and the exchange handler releases its resources immediately. The cap is configured via H2MultiplexingRequesterBootstrap and passed into H2MultiplexingRequester at construction time to keep requester configuration immutable and avoid API incompatibilities.

ok2c · 2025-12-28T09:42:27Z

@arturobernalg I do not like this approach. There is already a command queue built into the IOSession. We should leverage that queue instead of creating another layer of complexity. What we need is to extend IOSession interface to expose information about already pending commands and optionally reject commands if the queue is over a particular limit.

arturobernalg · 2025-12-29T18:07:07Z

@arturobernalg I do not like this approach. There is already a command queue built into the IOSession. We should leverage that queue instead of creating another layer of complexity. What we need is to extend IOSession interface to expose information about already pending commands and optionally reject commands if the queue is over a particular limit.

@ok2c . Sorry about the churn — I’m still getting up to speed on this part of the reactor stack and my previous commit took a wrong turn. I dropped the extra wrapper and enforce the cap via the existing IOSession command queue. I extended IOSession with a bounded enqueue + pending count and delegated through InternalDataChannel and SSLIOSession so the limit is not bypassed by decorators.

httpcore5/src/main/java/org/apache/hc/core5/reactor/IOSession.java

ok2c · 2025-12-30T16:52:36Z

httpcore5/src/main/java/org/apache/hc/core5/reactor/IOSession.java

+     * @return {@code true} if the command was enqueued, {@code false} otherwise.
+     * @since 5.5
+     */
+    default boolean enqueue(final Command command, final Command.Priority priority, final int maxPendingCommands) {


@arturobernalg I would not do that on the i/o session level. Different protocol handlers may have different restrictions. 10 concurrent requests may be perfectly OK for HTTP/2 and too much for HTTP/1.1. Please move this logic to individual protocol handlers.

@ok2c I just drop the IOSession-level enqueue/queue-limit API and keep the cap/reject logic strictly in the HTTP/2 requester/handler layer as requested.

...5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/bootstrap/H2MultiplexingRequester.java

ok2c

@arturobernalg Conceptually everything looks good to me. However as far as I can see you are also mixing in some code optimization into the same change-set.

Could you please do this?

Spin the optimization code into a separate pull request. It should get merged in first.
Re-base this pull request off it
Provide the same logic to HttpAsyncRequester for consistency.

arturobernalg · 2026-01-02T11:44:01Z

@arturobernalg Conceptually everything looks good to me. However as far as I can see you are also mixing in some code optimization into the same change-set.

Could you please do this?
1. Spin the optimization code into a separate pull request. It should get merged in first.

2. Re-base this pull request off it

3. Provide the same logic to `HttpAsyncRequester` for consistency.

@ok2c Please another pass

ok2c

@arturobernalg Almost there. Just a few comments.

ok2c · 2026-01-02T16:27:35Z

...5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/bootstrap/H2MultiplexingRequester.java

    private final H2ConnPool connPool;

+    /**
+     * Hard cap on per-connection queued / in-flight requests.


@arturobernalg Let's use the term "command" instead of "request" here to make sure there is no confusion with HTTP requests.

ok2c · 2026-01-02T16:28:40Z

...5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/bootstrap/H2MultiplexingRequester.java

+                            final int current = ioSession.getPendingCommandCount();
+                            if (current >= 0 && current >= max) {
+                                exchangeHandler.failed(new RejectedExecutionException(
+                                        "Maximum number of pending requests per connection reached (max=" + max + ")"));


@arturobernalg Same as above. Let say "command" instead of "request" here

ok2c · 2026-01-02T16:28:59Z

...main/java/org/apache/hc/core5/http2/impl/nio/bootstrap/H2MultiplexingRequesterBootstrap.java


    private IOReactorMetricsListener threadPoolListener;

+    private int maxRequestsPerConnection;


@arturobernalg Same.

ok2c · 2026-01-02T16:34:03Z

...src/test/java/org/apache/hc/core5/http2/examples/H2MaxRequestsPerConnectionLocalExample.java

@@ -0,0 +1,245 @@
+/*


@arturobernalg Let's drop all examples and integration tests. They look heavy and require a specially crafted server side listener. I wish we had unit test coverage for HttpAsyncRequester / H2MultiplexingRequester but we do not.

This functionality can have integration tests and examples in client along with other client request per connection policy configuration.

ok2c · 2026-01-02T20:41:23Z

httpcore5/src/test/java/org/apache/hc/core5/reactor/TestIOSessionImplMaxPendingCommands.java

+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+
+class TestIOSessionImplMaxPendingCommands {


@arturobernalg I am not sure what this test is actually testing. Let's drop it and the other test as well.

Restore per-session command count and use it in H2MultiplexingRequester to fail fast when the per-connection command queue exceeds the configured limit.

ok2c · 2026-01-02T21:40:38Z

...e/hc/core5/http2/impl/nio/bootstrap/TestH2MultiplexingRequesterMaxRequestsPerConnection.java

+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+class TestH2MultiplexingRequesterMaxRequestsPerConnection {


@arturobernalg This test should be moved to client and be made a part of a larger suite of related integration tests, but if you want to keep it here, so be it, but at least be consistent and provide a similar one for HttpAsyncRequester.

arturobernalg requested a review from ok2c December 27, 2025 18:14

arturobernalg force-pushed the max-requests-per-connection branch from de6f206 to b777f5d Compare December 29, 2025 17:58

ok2c requested changes Dec 30, 2025

View reviewed changes

arturobernalg force-pushed the max-requests-per-connection branch from 3cbecbe to 639e632 Compare December 30, 2025 18:53

arturobernalg requested a review from ok2c December 30, 2025 18:54

ok2c requested changes Jan 1, 2026

View reviewed changes

...5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/bootstrap/H2MultiplexingRequester.java Outdated Show resolved Hide resolved

arturobernalg force-pushed the max-requests-per-connection branch from 9c8911c to e6cb499 Compare January 1, 2026 13:20

arturobernalg requested a review from ok2c January 1, 2026 13:24

ok2c reviewed Jan 1, 2026

View reviewed changes

...5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/bootstrap/H2MultiplexingRequester.java Outdated Show resolved Hide resolved

ok2c reviewed Jan 1, 2026

View reviewed changes

arturobernalg force-pushed the max-requests-per-connection branch 7 times, most recently from 130dedf to 8aa41be Compare January 2, 2026 10:32

arturobernalg requested a review from ok2c January 2, 2026 11:28

ok2c requested changes Jan 2, 2026

View reviewed changes

arturobernalg force-pushed the max-requests-per-connection branch from 7ddd29a to 756d5a4 Compare January 2, 2026 18:11

arturobernalg requested a review from ok2c January 2, 2026 18:11

ok2c changed the title ~~Cap total number of concurrent requests per HTTP/2 connection~~ Cap total number of concurrent commands per connection Jan 2, 2026

ok2c reviewed Jan 2, 2026

View reviewed changes

Cap pending HTTP/2 request commands per connection

22d2ea5

Restore per-session command count and use it in H2MultiplexingRequester to fail fast when the per-connection command queue exceeds the configured limit.

arturobernalg force-pushed the max-requests-per-connection branch from 186431e to 22d2ea5 Compare January 2, 2026 21:25

arturobernalg requested a review from ok2c January 2, 2026 21:26

ok2c reviewed Jan 2, 2026

View reviewed changes


		private IOReactorMetricsListener threadPoolListener;

		private int maxRequestsPerConnection;

Cap total number of concurrent commands per connection #592

Are you sure you want to change the base?

Cap total number of concurrent commands per connection #592

Uh oh!

Conversation

arturobernalg commented Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ok2c commented Dec 28, 2025

Uh oh!

arturobernalg commented Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ok2c left a comment

Choose a reason for hiding this comment

Uh oh!

arturobernalg commented Jan 2, 2026

Uh oh!

ok2c left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

arturobernalg commented Dec 27, 2025 •

edited

Loading

arturobernalg commented Dec 29, 2025 •

edited

Loading